Skip to content

fix(config): qualify native Ollama exports after onboarding - #11550

Merged
prekshivyas merged 13 commits into
mainfrom
fix/e2e-attached-ollama-readiness
Sep 14, 2026
Merged

fix(config): qualify native Ollama exports after onboarding#11550
prekshivyas merged 13 commits into
mainfrom
fix/e2e-attached-ollama-readiness

Conversation

@sandl99

@sandl99 sandl99 commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Outcome

Native Linux Docker OpenClaw configuration export works with an attached Ollama daemon and refuses publication after that daemon stops. The GPU qualification passes alongside both existing inference scenarios.

Reason

Normal onboarding registers an internal proxy credential and an OpenAI provider type without a profile in its bound workspace. Export rejected those valid states. The E2E fixture also treated transient readiness timeouts as terminal, stopped its daemon before dependent cleanup, and rejected an internal hostname in the correctly preserved network policy.

Related issues

Refs #11435. Repairs export qualification introduced by #11473.

Changes

  • Record confirmed OpenAI-profile absence at global or provider-workspace bindings. Ollama export accepts that evidence; foreign bindings, failed reads, invalid profiles, and changing snapshots still refuse publication. Managed vLLM and Brave retain their profile requirements.
  • Require the managed proxy's internal credential key when onboarding selected no user credential. Verify the authenticated proxy and omit its credential from the exported document.
  • Onboard before attaching the fixture daemon. Use the shared proxy port, require service shutdown and model preparation, and bound readiness to 20 read-only probes for connection refusal or curl timeout. Restore the daemon for sandbox cleanup, then stop it.
  • Scope internal endpoint omission to inference providers while preserving explicit network policy and the whole-document credential-value check. Keep the live assertion budget, preserve readiness artifacts, and share existing export-test setup to stay within test-file size limits.

Verification

Tested commit: c9e6185488b3fc05bb0b125ca2f42280bd9cb555.

  • GPU E2E — all three scenarios and the managed image publication gate passed. The export scenario produced two matching specs, omitted the proxy credential, refused publication after daemon stop, and passed all seven cleanup steps. Retained artifact: 10335275785.
  • CI — passed, including all twelve CLI shards, static checks, package contracts, and type checks.
  • Local Ollama export suite — 16 passed after reproducing the policy-hostname assertion failure. The production repair passed 644 focused provider, export, verifier, and configuration-schema tests; unchanged GPU helper and polling suites passed 45 tests.
  • Live assertion census — 1787, unchanged. Normal commit and publication checks passed; all thirteen branch commits are GitHub Verified.
  • The diff contains no secrets, API keys, or credentials.

Review notes

Advisor completed all nine specialists for the tested commit: eight clear; one inherited timeout finding, F-delivery-flow-3fae8283e608b84d83d7.

Disposition: the recorded base already contains all three scenarios and their 75/90/75-minute limits. Its GPU job limit is already 90 minutes. This PR preserves those limits and reduces export setup from a 55-minute allowance to two 20-minute allowances. The finding is inherited and requires no change in this repair. The passing run does not eliminate that existing worst-case timeout risk.

Complete feedback collection found four comments, no reviews, and no review threads. CodeRabbit skipped the draft. No unresolved candidate-owned finding remains.

Self-review of NVIDIA/NemoClaw at the tested commit covered provider qualification, stable export evidence, proxy credential custody, the E2E lifecycle, and the owning inference/adapter guides against the shared security rubric. Export remains read-only and requires the authenticated proxy, daemon model, and managed OpenClaw route to agree. No approval or waiver is claimed. The PR remains draft.


Signed-off-by: San Dang sdang@nvidia.com
Signed-off-by: Julie Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

  • New Features

    • Ollama configuration exports now handle local credentials securely without exposing credential values.
    • Ollama routes can operate when an OpenAI profile is confirmed absent or appropriately scoped.
    • Export evidence now distinguishes between an absent profile and an unavailable profile.
  • Bug Fixes

    • Managed vLLM exports now refuse publication when the required OpenAI profile is missing or configuration has drifted.
    • Profile lookup failures other than confirmed absence continue to block exports.
  • Documentation

    • Added guidance for Ollama exports, profile handling, credential selection, and Brave profile onboarding.

@sandl99 sandl99 self-assigned this Sep 11, 2026
@copy-pr-bot

copy-pr-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d9d14da6-fe0f-433e-8fed-64d6034c36cc

📥 Commits

Reviewing files that changed from the base of the PR and between 31dc5b4 and c9e6185.

📒 Files selected for processing (15)
  • src/lib/adapters/config/live-export-source-ollama.test.ts
  • src/lib/adapters/config/live-export-source-test-fixture.ts
  • src/lib/adapters/config/live-export-source.test.ts
  • src/lib/adapters/config/live-export-source.ts
  • src/lib/adapters/openshell/README.md
  • src/lib/adapters/openshell/providers.test.ts
  • src/lib/adapters/openshell/providers.ts
  • src/lib/domain/config/export-evidence.ts
  • src/lib/domain/config/verify-ollama-serving.ts
  • src/lib/inference/README.md
  • test/e2e/README.md
  • test/e2e/live/gpu-e2e-helpers.ts
  • test/e2e/live/gpu-e2e.test.ts
  • test/e2e/support/gpu-e2e-helpers.test.ts
  • test/support/config-export-harness.ts

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Walkthrough

Walkthrough

The change adds nullable OpenAI profile handling, Ollama credential validation, shared configuration-export test utilities, attached Ollama export coverage, and revised GPU E2E daemon readiness and cleanup.

Changes

Ollama export qualification

Layer / File(s) Summary
Nullable OpenAI profile evidence
src/lib/adapters/openshell/providers.ts, src/lib/domain/config/export-evidence.ts, src/lib/domain/config/verify-ollama-serving.ts, src/lib/adapters/openshell/providers.test.ts, src/lib/adapters/openshell/README.md
OpenShell profile reads can return null for confirmed OpenAI profile absence. Ollama accepts eligible absent profiles, while managed vLLM requires a qualified profile.
Ollama export credential and evidence validation
src/lib/adapters/config/live-export-source.ts, src/lib/inference/README.md
Live export validation derives credential expectations from the route provider. Ollama local routes use the local credential contract, and null web-search profiles are omitted from evidence.
Shared export harness and refusal coverage
test/support/config-export-harness.ts, src/lib/adapters/config/live-export-source.test.ts, src/lib/adapters/config/live-export-source-ollama.test.ts, src/lib/adapters/config/live-export-source-test-fixture.ts
Configuration-export tests use shared mocks and helpers. New coverage checks Ollama export success, credential isolation, configuration drift, probe failures, missing attachments, unstable identities, and profile changes between snapshots.
Attached Ollama GPU E2E lifecycle
test/e2e/live/gpu-e2e-helpers.ts, test/e2e/live/gpu-e2e.test.ts, test/e2e/support/gpu-e2e-helpers.test.ts, test/e2e/README.md
The GPU E2E flow starts the fixture daemon after onboarding, waits for /api/tags, pulls the model, validates parsed export configuration, and restores the daemon for cleanup.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant GPU E2E test
  participant Attached Ollama
  participant Readiness helper
  participant Managed proxy
  GPU E2E test->>Attached Ollama: Start daemon after onboarding
  GPU E2E test->>Readiness helper: Poll /api/tags
  Readiness helper-->>GPU E2E test: Return readiness result
  GPU E2E test->>Attached Ollama: Pull configured model
  GPU E2E test->>Managed proxy: Export configuration
  Managed proxy-->>GPU E2E test: Return provider configuration and evidence
Loading

Possibly related PRs

  • NVIDIA/NemoClaw#11387: Introduces the managed profile evidence contract extended here to support confirmed absence.
  • NVIDIA/NemoClaw#11473: Adds the attached Ollama export path and GPU E2E lifecycle extended by this change.

Suggested reviewers: cv

Merge Risk: ⚪ Minimal · up to c9e61

No actionable merge-blocking issue is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 12 files. (3 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: qualifying native Ollama configuration exports after onboarding.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 4.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 12 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/e2e-attached-ollama-readiness

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit c9e6185 in the fix/e2e-attached-oll... branch remains at 96%, unchanged from commit be7c422 in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit c9e6185 in the fix/e2e-attached-oll... branch remains at 83%, unchanged from commit be7c422 in the main branch.

Show a line coverage summary of the most impacted files.
File main be7c422 fix/e2e-attached-oll... c9e6185 +/-
src/lib/onboard...uter-command.ts 65% 52% -13%
src/lib/onboard...outer-python.ts 96% 84% -12%
src/lib/adapter...xport-source.ts 95% 95% 0%
src/lib/adapter...ll/providers.ts 100% 100% 0%
src/lib/domain/...lama-serving.ts 100% 100% 0%
src/lib/onboard...file-builder.ts 94% 94% 0%
src/lib/sandbox...rce-identity.ts 82% 82% 0%
src/lib/onboard...uild-context.ts 74% 75% +1%
src/lib/domain/...aged-serving.ts 96% 100% +4%

Updated September 14, 2026 06:09 UTC

@sandl99 sandl99 changed the title fix(e2e): tolerate attached Ollama readiness timeouts fix(e2e): prepare attached Ollama for export Sep 11, 2026
@sandl99 sandl99 changed the title fix(e2e): prepare attached Ollama for export fix(config): export native Ollama without a provider profile Sep 14, 2026
Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 changed the title fix(config): export native Ollama without a provider profile fix(config): qualify native Ollama exports after onboarding Sep 14, 2026
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Review Advisor finished for commit c9e6185. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

All previous runs

@prekshivyas
prekshivyas marked this pull request as ready for review September 14, 2026 06:29
@prekshivyas
prekshivyas merged commit 71223c1 into main Sep 14, 2026
110 checks passed
@prekshivyas
prekshivyas deleted the fix/e2e-attached-ollama-readiness branch September 14, 2026 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants